Base path db constaint - #7952
Conversation
|
I looked at building an exclusion constraint. But as '^@' is not a symmetric operator, PG won't let me. |
|
And defining a new operator class is a sysadmin only trait in postgres. |
| slashed_path text := '/' || path || '/'; | ||
| BEGIN | ||
| -- Verify base path is normalized! | ||
| -- i.e. is relative, does not end with '/' and does not contain '//' or '.', '..' segments. |
There was a problem hiding this comment.
Do we also need to restrict the character set?
f2400a6 to
3ff6914
Compare
This allows to relieve the tasking system from taking care of base_path sanity.
The base_path integrity is maintained by a database constraint now.
3ff6914 to
c5cfa75
Compare
| def test_cannot_contain_another_base_path_as_prefix(self): | ||
| Distribution(name="0", base_path="a").save() | ||
| Distribution(name="1", base_path="a/a").save() | ||
| with pytest.raises(IntegrityError, match="prefix"): |
There was a problem hiding this comment.
Should we set match=prefixed since the error message between the two checks are different on this word?
There was a problem hiding this comment.
Actually, the "prefix" here ensures that the exception raised comes from the db trigger.
The order in which the two entries here are checked against the constraint is not necessarily intuitive. I think I was confused first to then realize:
PG first "inserts" both rows to then check the first before the second. So this will fail on "a" and not "a/a". But after all we just care that the transaction will be rejected.
Also, one thing I learned the hard way: Tests that rely on the literal error message are usually brittle. The important part of the check here is the "IntegrityError".
We even do not expect the user to see this often since for nicety/performance we still do the check in python first only reading the db.
2 participants
📜 Checklist
See: Pull Request Walkthrough